The centralization is valuable, and the migrated callers consistently propagate typed termination reasons and bounded stderr. I found one lifecycle bug in the new primitive that breaks the guarantee this PR is built around.

[P1] Do not treat every ChildProcess error as a spawn failure — packages/engine/src/utils/managedChildProcess.ts:102

Node emits ChildProcess error not only when spawn fails, but also when a running process cannot be killed (and for failed IPC sends / spawn-signal aborts). Today any such error immediately calls settle("spawn_error"). settle() then clears the SIGKILL escalation timer and removes the close listener (:159-176), while processTracker also drops the child on error (packages/engine/src/utils/processTracker.ts:5-10).

That means this concrete sequence leaks the process and lets the caller continue before reaping it:

1. deadline/abort calls kill("SIGTERM") and schedules SIGKILL (:134-156);
2. signal delivery fails and the child emits error while it can still be alive;
3. onError resolves wait() as spawn_error, cancels escalation, and removes the only close/reap listener.

The same premature settlement can occur for any post-spawn error, and kill() returning false is currently ignored. Please distinguish a true pre-spawn failure from runtime/termination errors. A post-spawn kill error must preserve the requested termination reason and the escalation/reap path (or otherwise synchronously prove the child is gone). Add a fake-child regression that aborts, emits error after SIGTERM, verifies wait() remains pending, observes SIGKILL after the grace period, and settles only on close.

This is directly documented by Node: the error event includes “process could not be killed,” and kill() returning true only means the signal was delivered—not that the process terminated.

Non-blocking scope note: packages/producer/src/services/audioExtractor.ts:84-105 remains a raw, unbounded FFmpeg lifecycle despite the PR body saying encode/probe/extract paths are migrated. If that path is intentionally outside this slice, narrowing the PR wording would avoid overstating coverage; otherwise it should use the shared primitive in a follow-up.

Verdict: REQUEST CHANGES

Reasoning: Exact head 1608c0e0cdaafa1763bebc7599dd1ffcf1f8fe16 is current and all required CI is green, but the new shared lifecycle can resolve and cancel escalation on a post-spawn kill error while the child remains alive. That violates the core cancellation/reap contract and affects every migrated caller.

— Magi

Audited: all 17 changed files in the complete main...a167618d2 diff, with the lifecycle primitive, tracker, every migrated caller, and tests read end-to-end. Trusting: no generated artifacts (none changed).

The original blocker is fixed correctly in packages/engine/src/utils/managedChildProcess.ts:94-118: a true pre-spawn failure settles as spawn_error, while a post-spawn runtime/kill error leaves escalation and the close-based reaping path active. packages/engine/src/utils/managedChildProcess.test.ts:61-91 pins the requested SIGTERM → error → SIGKILL → close sequence.

[P1] Remove tracked children on exit as well as close — packages/engine/src/utils/processTracker.ts:5-9

Changing the tracker from exit to only close fixes the post-spawn-error drop, but creates a different dangerous window. Node’s exit event means the child process has ended; close waits for stdio to close and can be delayed when another process still holds those streams. During that gap the dead child remains in tracked with proc.killed === false, so killTrackedProcesses() calls proc.kill("SIGTERM") (:15-24) and later SIGKILL. Node explicitly warns that signaling a child after it exited can target an unrelated process if the PID has already been reassigned.

The current tracker tests do not cover this: the test named “removes it after exit” actually waits for close (packages/engine/src/utils/processTracker.test.ts:11-18). Preserve the post-spawn-error fix without retaining exited PIDs by removing on both terminal events:

proc.once("exit", remove);
proc.once("close", remove); // spawn failures may have no exit event

Keep error non-terminal, and add a regression that emits exit without close, calls killTrackedProcesses(), and verifies kill is not invoked.

Exact head a167618d2eb4c8822ed8592a943c1d8901ff10d7 is mergeable. All completed required checks are green; six regression shards are still pending, with zero failures. This finding is independent of those pending jobs.

Verdict: REQUEST CHANGES

Reasoning: The managed-process fix now preserves escalation and reaping after runtime errors, but the tracker’s close-only cleanup can signal a stale/reused PID after the child has already exited. Removing on both exit and close closes that safety gap without reintroducing the original error-event bug.

— Magi

Audited: the complete 17-file PR diff in the prior rounds, all existing review/comment history, and the exact R3 delta at 018581b22c06d83aa94866dd0c1135a87b4898f3. Trusting: no unverified generated artifacts.

The original lifecycle blocker remains fixed: ManagedChildProcess distinguishes spawn failure from post-spawn runtime/kill errors, so those later error events do not settle wait(), cancel SIGKILL escalation, or detach the close-based reaping path. The fake-child lifecycle regression still pins abort → SIGTERM → post-spawn error → SIGKILL → close.

The follow-on tracker blocker is also resolved at the correct ownership boundary:

• packages/engine/src/utils/processTracker.ts:5-10 removes the tracked child on both exit and close, while leaving error non-terminal. This prevents shutdown from signaling a process that has exited but whose stdio has not closed, without regressing the reaping semantics.
• packages/engine/src/utils/processTracker.test.ts:21-37 reproduces that exact exit-without-close interval and verifies shutdown sends no signal to the exited child.
• The existing post-spawn-error coverage still verifies that an error alone does not remove a live child.

I found no remaining code findings. The exact head is current, open, and mergeable. Exact-head CI is still running with no failures; per James's request, this is the code verdict and does not treat pending CI as a failure.

Verdict: APPROVE
Reasoning: Both lifecycle ownership defects are fixed at their shared primitives and pinned by regressions that exercise the precise event-ordering failures; the R3 change introduces no new correctness issue.

— Magi